Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use exec to invoke the stage-2 bootstrap for non-zip case #2047

Merged
merged 3 commits into from
Jul 10, 2024

Conversation

chowder
Copy link
Contributor

@chowder chowder commented Jul 8, 2024

When the two-stage bootstrap is used, the parent shell process runs python as a child
process, which changes how signals are propagated. Specifically, if a signal is sent
directly to the parent (e.g. kill $parent), the child process (python) won't receive
it and it will appear to be ignored. This is because the parent process is busy waiting
for the child process.

To fix, invoke the python process using exec instead. Because the process is entirely
replaced, signals are sent directly to the replacement. This can't be used for zip files,
though, because they rely on a catching the exit signal to perform cleanup of the extracted
files.

Fixes #2043

Copy link

google-cla bot commented Jul 8, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@chowder chowder changed the title fix: use exec to invoke the stage-2 bootstrap in the common case fix: use exec to invoke the stage-2 bootstrap in the common case Jul 8, 2024
@rickeylev rickeylev changed the title fix: use exec to invoke the stage-2 bootstrap in the common case fix: use exec to invoke the stage-2 bootstrap for non-zip case Jul 8, 2024
Also remove a defunct comment since we're here
Copy link
Contributor

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

#2045 adds the basics for testing this, so we can put a test in after that lands.

@rickeylev rickeylev enabled auto-merge July 10, 2024 02:28
@rickeylev rickeylev disabled auto-merge July 10, 2024 02:30
@rickeylev rickeylev enabled auto-merge July 10, 2024 02:33
@rickeylev rickeylev added this pull request to the merge queue Jul 10, 2024
Merged via the queue into bazelbuild:main with commit e690975 Jul 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Linux) The stage-1 bootstrap script does not propagate certain signals to the stage-2 bootstrap script.
2 participants